Radio and checkbox icon buttons #1892

blackbaud-conorwright · 2018-08-10T13:40:44Z

Created new radio and checkbox icon button components.

The tag names are up for debate, I'm still not sure if sky-checkbox-icon and sky-radio-icon are clear enough. They seem like they could be confused with an icon component that can be placed inside of these elements.

Resolves: #970

Original contribution: #1644

…on-buttons

codecov-io · 2018-08-10T20:11:44Z

Codecov Report

❗ No coverage uploaded for pull request base (master@dfbf923). Click here to learn what that means.
The diff coverage is 100%.

@@           Coverage Diff            @@
##             master   #1892   +/-   ##
========================================
  Coverage          ?    100%           
========================================
  Files             ?     424           
  Lines             ?    8968           
  Branches          ?    1326           
========================================
  Hits              ?    8968           
  Misses            ?       0           
  Partials          ?       0

Impacted Files	Coverage Δ
src/modules/checkbox/checkbox.component.ts	`100% <100%> (ø)`
src/modules/radio/radio.component.ts	`100% <100%> (ø)`
src/modules/radio/radio.module.ts	`100% <100%> (ø)`
src/modules/checkbox/checkbox.module.ts	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfbf923...1d3f2f4. Read the comment docs.

blackbaud-johnly · 2018-08-15T01:31:50Z

Created #1907 to document new components.

Blackbaud-AlexKingman

Solid work here. Just a few questions/comments. Also, I don't necessarily have a problem with the component names. I couldn't really think of an alternative that was better, so I think I'm happy with what you've got here. Great job.

Blackbaud-AlexKingman · 2018-08-15T15:08:03Z

src/demos/checkbox/checkbox-demo.component.ts

@@ -25,4 +25,34 @@ export class SkyCheckboxDemoComponent {
      disabled: true
    }
  ];
+  public iconCheckboxItems = [


I noticed none of these are disabled. Should we show at least one disabled value to show consumers how to do it?

Yeah, that's a good plan. I'm going to add strikethrough to the group example.

Blackbaud-AlexKingman · 2018-08-15T15:38:56Z

src/scss/_forms.scss

@@ -118,3 +153,23 @@ fieldset {
  flex: 1 1 auto;
  width: 100%;
 }
+
+.sky-switch-icon-group{


Where is sky-switch-icon-group being used? I can't find it anywhere in our codebase.

Sorry, I should have messaged you. Todd just added this as a class that people can put on the container of their icon buttons (radio or checkbox) to have them appear as an inline group. I'm adding an example to each demo now

Ah cool! That will be helpful. I noticed there's some style inconsistencies with this block of CSS - space before the bracket and some tabs needed. Other than that, I'm good with this 👍

Blackbaud-AlexKingman · 2018-08-15T15:47:49Z

src/modules/checkbox/checkbox-icon.component.html

+      'sky-switch-control-danger': checkboxType === 'danger'
+    }">
+    <sky-icon
+      aria-hidden="true"


I don't think you need aria-hidden anymore with the new icon component, do you?

This is true. I do not need this anymore 👍
Removing.

Blackbaud-AlexKingman · 2018-08-15T15:53:03Z

src/modules/checkbox/checkbox-icon.component.ts

+    return this._checkboxType || 'info';
+  }
+  public set checkboxType(value: string) {
+    this._checkboxType = value;


Do you want to also do a if(value) check and a .toLowerCase() here?

Yes, I do want that. Added

Blackbaud-AlexKingman · 2018-08-15T15:55:44Z

src/modules/checkbox/checkbox-icon.component.ts

+  useExisting: forwardRef(() => SkyCheckboxIconComponent),
+  multi: true
+};
+


Should we have a // tslint:enable here?

oh yeah, nice catch

Blackbaud-AlexKingman · 2018-08-15T15:58:09Z

src/modules/checkbox/checkbox.component.html

+  <span
+    class="sky-switch-control sky-rounded-corners">
+    <sky-icon
+      *ngIf="checked"
      aria-hidden="true"


Same comment as before - can you get rid of aria-hidden now?

Blackbaud-AlexKingman · 2018-08-15T16:02:51Z

src/modules/radio/radio-icon.component.spec.ts

+  public radioType: string;
+}
+
+describe('Icon radio component', () => {


Should this say Radio icon component ? I only ask because if we're quickly scanning the list of tests, you might have trouble finding it since its not consistent with the component name.

That's a good point. I'll switch them around

Blackbaud-AlexKingman · 2018-08-15T16:03:21Z

src/modules/checkbox/checkbox-icon.component.spec.ts

+  public checkboxType: string;
+}
+
+describe('Icon checkbox component', () => {


Same comment as before - Checkbox icon component.

blackbaud-conorwright · 2018-08-15T18:53:25Z

updated @Blackbaud-AlexKingman 👍

…yux2 into radio-checkbox-icon-buttons

Blackbaud-AlexKingman · 2018-08-15T19:47:44Z

Talked offline about not using rounded corners in the groups. Once thats done, I'm good to go with this!

blackbaud-conorwright · 2018-08-16T13:36:47Z

@Blackbaud-AlexKingman I've updated the demos to use the groupings properly. They were already set, the icon components have to be direct children of the switch-group element.

Blackbaud-AlexKingman · 2018-08-16T13:49:23Z

Smooth. I like it 🚢 🇮🇹 !

blackbaud-conorwright · 2018-08-27T16:06:15Z

On hold for: blackbaud/skyux-theme#23

…on-buttons

blackbaud-conorwright and others added 9 commits August 3, 2018 14:06

initial component scaffolding

d445d96

Merge remote-tracking branch 'upstream/master' into radio-checkbox-ic…

016df73

…on-buttons

created initial icon functionality

99740f4

increased size. Added colors

7177c03

finished styling. Added testing

5f406f5

Merge branch 'master' into radio-checkbox-icon-buttons

0ab7e35

Merge remote-tracking branch 'upstream/master' into radio-checkbox-ic…

2d656fd

…on-buttons

added fixed radio icon comp import

44b19ff

added labels to the visual tests

fb2c56a

blackbaud-conorwright requested a review from Blackbaud-AlexKingman August 13, 2018 14:19

Blackbaud-SteveBrush assigned Blackbaud-AlexKingman Aug 14, 2018

blackbaud-conorwright added the Status: Ready for review label Aug 14, 2018

Blackbaud-SteveBrush mentioned this pull request Aug 14, 2018

Added icon style checkboxes #1644

Closed

override max width

d12d130

Blackbaud-ToddRoberts and others added 2 commits August 15, 2018 09:07

add css to handle grouped icon buttons

4b4f96f

Merge branch 'master' into radio-checkbox-icon-buttons

7ba4468

Blackbaud-AlexKingman suggested changes Aug 15, 2018

View reviewed changes

blackbaud-conorwright and others added 2 commits August 15, 2018 14:49

added group examples. Addressed comments

d41db87

Merge branch 'master' into radio-checkbox-icon-buttons

0fa4333

blackbaud-conorwright added 3 commits August 15, 2018 14:57

fixed scss spacing

8ba3989

Merge branch 'radio-checkbox-icon-buttons' of github.com:blackbaud/sk…

d4a8787

…yux2 into radio-checkbox-icon-buttons

fixed spacing issue

6674ff8

updated demos to have switch group displays

b3d2659

blackbaud-conorwright mentioned this pull request Aug 16, 2018

Create Icon radio button and icon checkbox documentation #1914

Closed

Blackbaud-AlexKingman added Status: Ready to merge and removed Status: In progress labels Aug 24, 2018

Blackbaud-AlexKingman assigned blackbaud-conorwright and unassigned Blackbaud-AlexKingman Aug 27, 2018

This was referenced Aug 27, 2018

added switch-input icon styles blackbaud/skyux-theme#22

Closed

added switch-input icon styles blackbaud/skyux-theme#23

Merged

blackbaud-conorwright added Status: On hold and removed Status: Ready to merge labels Aug 27, 2018

blackbaud-conorwright and others added 3 commits August 27, 2018 13:37

Merge remote-tracking branch 'upstream/master' into radio-checkbox-ic…

da0c651

…on-buttons

updated skyux theme and made it take all minor revisions

bc35a2d

Merge branch 'master' into radio-checkbox-icon-buttons

8b3aba6

blackbaud-conorwright added Status: Ready to merge Status: Ready for review and removed Status: On hold Status: Ready to merge labels Aug 28, 2018

Change theme dependency to use exact version

cc92df7

Blackbaud-SteveBrush added the From: Consumer label Aug 29, 2018

blackbaud-conorwright and others added 3 commits August 29, 2018 10:26

added theme dependency to visual tests

28560a9

fixed visual tests for icon switches

7a209e9

Merge branch 'master' into radio-checkbox-icon-buttons

6b0754d

blackbaud-conorwright added Status: Ready to merge sky-checkbox Severity: Medium and removed Status: Ready for review labels Aug 29, 2018

blackbaud-conorwright and others added 2 commits August 29, 2018 14:34

Merge branch 'master' into radio-checkbox-icon-buttons

ea856f9

Merge branch 'master' into radio-checkbox-icon-buttons

1d3f2f4

blackbaud-conorwright merged commit e02f5b4 into master Aug 29, 2018

blackbaud-conorwright deleted the radio-checkbox-icon-buttons branch August 29, 2018 19:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Radio and checkbox icon buttons #1892

Radio and checkbox icon buttons #1892

blackbaud-conorwright commented Aug 10, 2018 •

edited

Loading

codecov-io commented Aug 10, 2018 •

edited

Loading

blackbaud-johnly commented Aug 15, 2018

Blackbaud-AlexKingman left a comment

Blackbaud-AlexKingman Aug 15, 2018

blackbaud-conorwright Aug 15, 2018

Blackbaud-AlexKingman Aug 15, 2018

blackbaud-conorwright Aug 15, 2018

Blackbaud-AlexKingman Aug 15, 2018

Blackbaud-AlexKingman Aug 15, 2018

blackbaud-conorwright Aug 15, 2018

Blackbaud-AlexKingman Aug 15, 2018

blackbaud-conorwright Aug 15, 2018

Blackbaud-AlexKingman Aug 15, 2018

blackbaud-conorwright Aug 15, 2018

Blackbaud-AlexKingman Aug 15, 2018

blackbaud-conorwright Aug 15, 2018

Blackbaud-AlexKingman Aug 15, 2018

blackbaud-conorwright Aug 15, 2018

Blackbaud-AlexKingman Aug 15, 2018

blackbaud-conorwright Aug 15, 2018

blackbaud-conorwright commented Aug 15, 2018

Blackbaud-AlexKingman commented Aug 15, 2018

blackbaud-conorwright commented Aug 16, 2018

Blackbaud-AlexKingman commented Aug 16, 2018

blackbaud-conorwright commented Aug 27, 2018

Radio and checkbox icon buttons #1892

Radio and checkbox icon buttons #1892

Conversation

blackbaud-conorwright commented Aug 10, 2018 • edited Loading

codecov-io commented Aug 10, 2018 • edited Loading

Codecov Report

blackbaud-johnly commented Aug 15, 2018

Blackbaud-AlexKingman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blackbaud-conorwright commented Aug 15, 2018

Blackbaud-AlexKingman commented Aug 15, 2018

blackbaud-conorwright commented Aug 16, 2018

Blackbaud-AlexKingman commented Aug 16, 2018

blackbaud-conorwright commented Aug 27, 2018

blackbaud-conorwright commented Aug 10, 2018 •

edited

Loading

codecov-io commented Aug 10, 2018 •

edited

Loading